Skip to content

Record all removed vars in novel_train - #33

Merged
rcannood merged 2 commits into
mainfrom
fix/novel-removed-vars
Jul 29, 2026
Merged

Record all removed vars in novel_train#33
rcannood merged 2 commits into
mainfrom
fix/novel-removed-vars

Conversation

@rcannood

Copy link
Copy Markdown
Member

Describe your changes

novel_train drops every all-zero feature from the training matrix:

zero_row = input_train_mod1.X.sum(axis=0) == 0
rem_var = None
if True in zero_row:
  rem_var = input_train_mod1[:, zero_row].var_names
  input_train_mod1 = input_train_mod1[:, ~zero_row]

but only hands the first one to novel_predict:

adata.uns["removed_vars"] = [rem_var[0]]

so novel_predict removes exactly one column from the test matrix regardless of how many the model dropped. With two or more all-zero features the test matrix is wider than the model expects -- the TF-IDF idf vector no longer broadcasts, or nn.Linear gets the wrong input size on the ADT path:

all-zero training features: ['g1', 'g3']
  old: [rem_var[0]]      -> test has 4 features, model expects 3 <-- MISMATCH
  new: list(rem_var)     -> test has 3 features, model expects 3 OK

The comment says "Mostly only applicable for testing", but nothing stops a real GEX training split from having several all-zero genes.

Part of a series of PRs coming out of a pre-run review of the benchmark.

Checklist before requesting a review

  • I have performed a self-review of my code

  • Check the correct box. Does this PR contain:

    • Breaking changes
    • New functionality
    • Major changes
    • Minor changes
    • Bug fixes
  • Proposed changes are described in the CHANGELOG.md

  • CI Tests succeed and look good!

rcannood added 2 commits July 29, 2026 09:47
rem_var holds every all-zero training feature, but only rem_var[0] was
stored, so novel_predict removed one column from the test matrix no
matter how many the model had dropped.
@rcannood
rcannood merged commit b6f7ad5 into main Jul 29, 2026
1 check passed
@rcannood
rcannood deleted the fix/novel-removed-vars branch July 29, 2026 12:11
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant